feat: Updates for osmosis v17 #5
Conversation
| default = ["osmosis-test-tube"] | ||
| # for more explicit tests, cargo test --features=backtraces | ||
| backtraces = ["cosmwasm-std/backtraces"] | ||
| # use library feature to disable all instantiate/execute/query exports | ||
| library = [] | ||
| osmosis-test-tube = ["cw-it/osmosis-test-tube", "apollo-vault-test-helpers/osmosis-test-tube"] |
There was a problem hiding this comment.
why do we need this? can we not just import these crates with the osmosis-test-tube feature by default?
There was a problem hiding this comment.
The only reason is because debugging does not work when cw-it/osmosis-test-tube is enabled.
| @@ -32,22 +34,22 @@ optimize = """docker run --rm -v "$(pwd)":/code \ | |||
|
|
|||
| [dependencies] | |||
| apollo-vault = {path = "../../packages/apollo-vault", features = ["lockup", "force-unlock"], default-features = false } | |||
| osmosis-std = { git = "https://github.com/osmosis-labs/osmosis-rust.git", rev = "7c1d418" } | |||
| osmosis-std = "0.17.0-rc0" | |||
| let dependencies = | ||
| OsmosisVaultRobot::instantiate_deps(&runner, &admin, DEPENDENCY_ARTIFACTS_DIR); |
There was a problem hiding this comment.
why not call this in the with_single_rewards function?
There was a problem hiding this comment.
I guess we don't need it right now but I used this pattern in locked-astroport-vault because in the tests for neutron-reward-distributor I needed to be able to instantiate two vaults with the same dependency contracts. Figured its a more re-usable pattern even if we don't need it right now.
| /// Tests that the first deposit works when there are already some reward assets to be compounded | ||
| /// in the vault before the first deposit. This was a previous bug that has been fixed. |
There was a problem hiding this comment.
how do we end up in the scenario where there are rewards prior to the first deposit? is it user1 someone enters, rewards accrue, and then user1 emergency withdraws all and so compounding is not triggered?
There was a problem hiding this comment.
Rewards can end up in the vault by someone just sending them directly to the vault without calling ExecuteMsg::Deposit. This is usually not done, but in this case this bug was discovered because Osmosis removed the denom creation fee, which means that the 10 OSMO that were sent to the contract upon instantiation were in there before the first deposit.
| let vault_token_denom = vault_token.to_string(); | ||
|
|
||
| // Check that exactly the right funds were sent | ||
| if info.funds.len() != 1 | ||
| || info.funds[0].denom != vault_token_denom | ||
| || info.funds[0].amount != vault_token_amount | ||
| { | ||
| return Err(ContractError::UnexpectedFunds { | ||
| expected: vec![Coin { | ||
| denom: vault_token.to_string(), | ||
| amount: vault_token_amount, | ||
| }], | ||
| actual: info.funds.clone(), | ||
| }); | ||
| } |
There was a problem hiding this comment.
is this not handled by the vault_token.receive?
There was a problem hiding this comment.
It doesn't check if there are extra denoms in info.funds. I figured we might as well make it explicit that we only want exactly the right funds. Can refactor this into a helper function later on.
| let contract = ContractType::Artifact(cw_it::Artifact::Local(wasm_file_path.to_string())); | ||
|
|
||
| let code_id = upload_wasm_file(app, &admin, contract).unwrap(); | ||
|
|
||
| let config = ConfigUnchecked { | ||
| performance_fee: Decimal::percent(3), //TODO: variable performance fee | ||
| treasury: treasury.address(), | ||
| // TODO: Setup router | ||
| router: CwDexRouterUnchecked::new(app.init_account(&[]).unwrap().address()), | ||
| router: dependencies | ||
| .cw_dex_router_robot | ||
| .cw_dex_router | ||
| .clone() | ||
| .into(), | ||
| reward_assets: vec![AssetInfoUnchecked::native( | ||
| reward_pool.liquidity[0].denom.clone(), | ||
| )], | ||
| reward_liquidation_target: AssetInfoUnchecked::native( | ||
| base_pool.liquidity[0].denom.clone(), | ||
| ), | ||
| force_withdraw_whitelist: vec![fwa_admin.address()], | ||
| // TODO: Setup liquidity helper | ||
| liquidity_helper: LiquidityHelperUnchecked::new( | ||
| app.init_account(&[]).unwrap().address(), | ||
| ), | ||
| liquidity_helper: dependencies.liquidity_helper.clone().into(), |
There was a problem hiding this comment.
Good point. I guess our current tests are a bit lacking since they just use reward denoms that are part of the base pool.
|
|
||
| pub const CW_DEX_ROUTER_WASM_NAME: &str = "cw_dex_router_osmosis.wasm"; | ||
|
|
||
| pub struct CwDexRouterRobot<'a> { |
There was a problem hiding this comment.
this is used in many of our vaults. this is ok for now but long term we should put it in the dex-router crate asA helper
No description provided.